-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CouchDB physical backend #2880
CouchDB physical backend #2880
Conversation
It's important to note that while I'm super happy to have couchdb, it's not safe to merely run multiple Vaults on top of replicated underlying data, so this is not a "replication" mechanism for Vault. From past discussions I know that CouchDB doesn't support transactions. However, if it implements the PseudoTransactional interface then you could build a transactional variant on top using built-in primitives in Vault's physical layer. It's not true transactions and it's slow, but it would allow Vault Enterprise users to use CouchDB underneath Vault's replication. |
I've implemented the @jefferai any other things you'd want corrected for this PR to land? |
physical/couchdb.go
Outdated
|
||
// GetInternal is used to fetch an entry | ||
func (m *TransactionalCouchDBBackend) GetInternal(key string) (*Entry, error) { | ||
return m.Get(key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is backwards; the entire point of the 'internal' functions are that they don't acquire a permit.
Rather than GetInternal chaining to Get, Get should chain to GetInternal where the heavy lifting happens. Both Get and GetInternal should be methods of the base CouchDBBackend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for the explanation - I've corrected this now.
@nicolai86 Note, if you look into making it HA, that locks need sessions, or some other heartbeat/timeout mechanism. Otherwise if manual recovery is necessary every time the active node goes down, you've defeated the purpose. |
physical/couchdb.go
Outdated
endpoint: endpoint, | ||
username: username, | ||
password: password, | ||
Client: &http.Client{}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use cleanhttp.DefaultPooledClient
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just that one comment, other than that looks good.
Thanks! |
yay, thank you! |
* oss/master: (161 commits) update gitignore changelog++ Exclude /sys/leases/renew from registering with expiration manager (#2891) More cleanup Clarify/fix some configuration info. Add a convenience function for copying a client (#2887) Better error messages using ListObjects than using HeadBucket. Might be a bigger request but messages are better than BadRequest, how this changes effect the messages are in the issue (#2892) Add ACL info to Consul configuration page Return error on bad CORS and add Header specification to API request primitive Add Zyborg.Vault PowerShell module to libs list (#2869) changelog++ CouchDB physical backend (#2880) Fix root paths test Add missing datadog vendored lib changelog++ Fix up CORS. Cors headers (#2021) Address review feedback Fix the test error message Added utility on router to fetch mount entry using its ID ...
couchdb is a great storage system when your requirements include a bullet proof replication mechanism.
this PR allows vault to be configured to persist it's configuration in couchdb like this:
then the database administrator can use couchdb's build in replication mechanism to configure eventual consistency for Vaults configuration.
The
PseudoTransactional
interface is available via thecouchdb_transactional
storage.included in this PR
PseudoTransactional
interfacethe backend deliberatly does not handle conflicts (yet) to keep it simple